Skip to content

Conversation

@RotRotAl
Copy link

@RotRotAl RotRotAl commented Sep 25, 2025

Allows to override the client tags that were set in the data source addition , via the the query editor.
Based on pr #309 by @laserninja that allows setting the client tags in the data source addition .

@cla-bot
Copy link

cla-bot bot commented Sep 25, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@RotRotAl
Copy link
Author

RotRotAl commented Oct 21, 2025

Cla was accepted waiting for #309 before i can publish this @laserninja @RoeyoOgen

return ds.SQLDatasource.QueryData(ctx, req)
}

func (ds *SQLDatasourceWithTrinoUserContext) injectClientTags(contextWithTags context.Context, req *backend.QueryDataRequest, settings models.TrinoDatasourceSettings) context.Context {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not referencing ds anywhere, so this method doesn't need to be bound to it. Remove (ds *SQLDatasourceWithTrinoUserContext), and place it after injectAccessToken.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look done, have you not pushed some changes?

@nineinchnick
Copy link
Member

Please squash all commits

@laserninja
Copy link
Contributor

@RotRotAl can you please create a PR with just your changes? We can get this approved #309 meanwhile

@RotRotAl
Copy link
Author

@laserninja sadly my code based on your changes, i will pull your changes after you finish the cr process

@cla-bot cla-bot bot added the cla-signed label Oct 28, 2025
@RotRotAl RotRotAl force-pushed the feature/ticketless-client-tags-in-query-editor branch 2 times, most recently from 904a534 to c699b9e Compare October 29, 2025 13:05
@RotRotAl RotRotAl requested a review from nineinchnick October 29, 2025 13:10
@RotRotAl
Copy link
Author

@nineinchnick done replying and fixing, thanks for the cr.

@RotRotAl RotRotAl force-pushed the feature/ticketless-client-tags-in-query-editor branch from 8bb3509 to 61c6a88 Compare October 29, 2025 13:25
@RotRotAl
Copy link
Author

RotRotAl commented Nov 2, 2025

Hi @nineinchnick stil needs second cr 🙏🏻

@RotRotAl
Copy link
Author

@nineinchnick still waits for cr

Copy link
Member

@nineinchnick nineinchnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the PR description with some examples of when this is useful? I'm not sure why this is needed over creating multiple data sources with different client tags configured.

await expect(page.getByTestId('data-testid table body')).toContainText(/.*1995-01-19 0.:00:005703857F.*/);
}

async function runQueryAndRetrunRequset(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the typos

import { DataSource } from './datasource';
import { TrinoDataSourceOptions, TrinoQuery, defaultQuery, SelectableFormatOptions } from './types';
import { FormatSelect, QueryCodeEditor } from '@grafana/aws-sdk';
import { Input } from '@grafana/ui'; // <-- ADD THIS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the comment for?

@RotRotAl
Copy link
Author

I need it in order to give my “clients” the ability to choose in which cluster there query should be running, i give them only the gateway link and they decide by client tag where will it run. @nineinchnick

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants